Skip to content

Add OffDiagonal, returned when slicing Diagonal fills and RectDiagonal - #454

Merged
dlfivefifty merged 2 commits into
masterfrom
offdiagonal
Oct 9, 2026
Merged

dlfivefifty merged 2 commits into
masterfrom
offdiagonal

Conversation

@dlfivefifty

Copy link
Copy Markdown
Member

Adds FillArrays.OffDiagonal, a matrix whose only nonzero entries lie on a single diagonal:

julia> FillArrays.OffDiagonal([1, 2, 3], 1)
4×4 FillArrays.OffDiagonal{Int64, Vector{Int64}, Tuple{Base.OneTo{Int64}, Base.OneTo{Int64}}}:
 ⋅  1  ⋅  ⋅
 ⋅  ⋅  2  ⋅
 ⋅  ⋅  ⋅  3
 ⋅  ⋅  ⋅  ⋅

OffDiagonal(band, k, axes) stores band on the k-th diagonal, using the same convention as diag/diagind (k > 0 is above the diagonal). length(band) must match the length of that diagonal, and may be zero if the diagonal lies outside the matrix. OffDiagonal(band, k) builds the square length(band) + abs(k) matrix.

Slicing

Indexing these types by unit ranges or : now returns an OffDiagonal instead of a dense Matrix:

  • Diagonal{<:Number,<:AbstractFillVector} (including square Eye)
  • any RectDiagonal (including rectangular Eye)
  • OffDiagonal itself

A slice of any of these matrices has nonzeros on at most one diagonal, so the result needs only a new band offset and a slice of the band. Lazy bands stay lazy, and the slice doesn't allocate:

julia> Eye(5)[2:4, 1:3] === FillArrays.OffDiagonal(Ones(2), 1, 3, 3)
true

julia> @allocated Eye(5)[2:4, 1:3]
0

What still takes the existing path:

  • Diagonals not backed by a fill, such as Diagonal([1, 2, 3])[1:2, 1:2], still return a Matrix.
  • Indices whose result would have offset or infinite axes fall back to the generic getindex.
  • Diagonal fills with non-Number eltypes (block diagonals) are left alone, because OffDiagonal fills off-band entries with zero(T).

OffDiagonal also gets setindex! (on the band only), diag, ⋅ printing for zeros, and hashing alongside RectDiagonal. Like RectDiagonal, it isn't exported.

The version is bumped to 1.18.0, since these slices now return a different type.

Tests

The new testset checks every contiguous sub-range of a set of Eye, Diagonal fill, RectDiagonal and OffDiagonal matrices against dense slicing: square and rectangular, Vector and Fill bands, sub- and superdiagonals, and empty bands.

🤖 Generated with Claude Code

…gonal

`OffDiagonal(band, k, axes)` is a matrix whose only nonzero entries lie on
its `k`-th diagonal, stored as `band`. Indexing a `Diagonal` of an
`AbstractFill`, any `RectDiagonal`, or an `OffDiagonal` by unit ranges or
colons now returns an `OffDiagonal` instead of a dense `Matrix`, so e.g.
`Eye(5)[2:4, 1:3]` stays lazy and allocation-free.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@codecov

codecov Bot commented Oct 4, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (466c058) to head (558cbc3).
⚠️ Report is 4 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##            master      #454   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           10        11    +1     
  Lines         1259      1316   +57     
=========================================
+ Hits          1259      1316   +57     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Covers the fallback to the generic getindex for indices whose axes are not
Base.OneTo, which returns a static matrix.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@dlfivefifty

Copy link
Copy Markdown
Member Author

The motivation for this is so that eg Eye(n)[1:n,2:n] will return a sparse type. But actually I'm planning to use this in ArrayLayouts.jl for all Diagonal{T, <:LayoutArray}.

The only potential downside is if someone expects slicing to make a mutable array. But we already break that assumption for Fill etc. so I think it's OK. And my view if it doesn't break any unit tests / integration tests then it doesn't count as breaking.

@dlfivefifty
dlfivefifty requested a review from jishnub October 5, 2026 20:35
@dlfivefifty
dlfivefifty merged commit 3f8b7e9 into master Oct 9, 2026
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant